Skip to content

feat(bob): concurrent swaps#971

Open
binarybaron wants to merge 16 commits into
masterfrom
feat/bob-concurrent-swaps
Open

feat(bob): concurrent swaps#971
binarybaron wants to merge 16 commits into
masterfrom
feat/bob-concurrent-swaps

Conversation

@binarybaron
Copy link
Copy Markdown

@binarybaron binarybaron commented Apr 28, 2026

No description provided.

@binarybaron
Copy link
Copy Markdown
Author

We should display the final state of the swap until the user acknowledges it.

@binarybaron
Copy link
Copy Markdown
Author

We should move the "cancel timeline" from the "history" page to the "swaps" page and hide it somewhat (make it expandable).

@binarybaron
Copy link
Copy Markdown
Author

Issue:

run_exclusive_initiation releases the initiation lock at swap_manager.rs:642 before the lock-tx is broadcast. UTXO selection happens inside the spawned task (TxLock::new → wallet.send_to_address); the wallet mutex is method-level, so two concurrent buy_xmr calls can build PSBTs from the same UTXOs.

@Einliterflasche
Copy link
Copy Markdown

Einliterflasche commented May 18, 2026

The interaction with the Watcher needs to be updated. The watcher still does in-line cancel_and_refund but should use the swap manager now. At least prevent the swap manager from spawning another state machine while the watcher is refunding

@binarybaron
Copy link
Copy Markdown
Author

The interaction with the Watcher needs to be updated. The watcher still does in-line cancel_and_refund but should use the swap manager now. At least prevent the swap manager from spawning another state machine while the watcher is refunding

Yes you are right but the whole watcher should become redundant anyway... It should not be required if the state machine is properly working. The watcher should be abolished at some point.

@Einliterflasche
Copy link
Copy Markdown

Einliterflasche commented May 18, 2026

I have 2 swaps where I had to do the XMR redeem manually which are now always automatically resumed - and keep failing. There needs to be a solution to this. Some kind of manually marking swaps as "do not resume" which is persisted across restarts.

Apart from that, I think we should change "Suspend" in the GUI to something like "Stop" or "Halt".

@binarybaron
Copy link
Copy Markdown
Author

I have 2 swaps where I had to do the XMR redeem manually which are now always automatically resumed - and keep failing. There needs to be a solution to this. Some kind of manully marking swaps as "do not resume" which is persisted across restarts.

Apart from that, I think we should change "Suspend" in the GUI to something like "Stop" or "Halt".

"Stop" or "Halt" seems like this actually stop the timelocks from counting down or does something on a protocol level. I dont see them being clearer than "suspend".

@Einliterflasche
Copy link
Copy Markdown

"Stop" or "Halt" seems like this actually stop the timelocks from counting down or does something on a protocol level. I dont see them being clearer than "suspend".

Suspend has the same issue, except I have a much harder time imagining what "suspend" means. It's technical jargon.
We need to warn anyway that this doesn't stop the swap / refund window from progressing.

@Einliterflasche
Copy link
Copy Markdown

I'll remove the watcher.

What do you propose for the manually-finished-swap problem? We could just add a new db table with columns like swap_id, should_not_resume.

@binarybaron
Copy link
Copy Markdown
Author

I'll remove the watcher.

What do you propose for the manually-finished-swap problem? We could just add a new db table with columns like swap_id, should_not_resume.

We could also bundle it with #932

@binarybaron
Copy link
Copy Markdown
Author

Not sure if removing the watcher is the move yet... its a reasonable safety fallback mechanism.

@Einliterflasche
Copy link
Copy Markdown

Einliterflasche commented May 18, 2026

It also turns out we use the watcher for balance and timelock updates. I'll keep the watcher and make it interact with the swap manager.

We could also bundle it with #932

Yes. Although there still should be an option to stop a swap from resuming without fully deleting it. It needs to be done as part of this PR though. Otherwise it gets really annoying for people in this situation of which there are a few.

@binarybaron binarybaron marked this pull request as ready for review May 19, 2026 10:01
@binarybaron
Copy link
Copy Markdown
Author

@Einliterflasche I will merge this today.

Comment thread swap/src/database/sqlite.rs
Comment thread swap/src/cli/swap_manager.rs
Comment thread swap/src/cli/api/request.rs
Comment thread src-gui/src/store/hooks.ts
@Einliterflasche
Copy link
Copy Markdown

We definitely need a docker test which spawns multiple swaps. Also get the ui mock system to work again, it's pretty broken now.

@binarybaron
Copy link
Copy Markdown
Author

Also get the ui mock system to work again, it's pretty broken now.

Can you expand? It works perfectly fine for me.

Comment thread src-gui/src/renderer/components/pages/swap/swap/CancelButton.tsx
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 90c4b2f. Configure here.

Comment thread swap/src/cli/watcher.rs
// If the swap is already running, we can skip the refund
// The refund will be handled by the state machine
if let Some(current_swap_id) = self.swap_lock.get_current_swap_id().await {
if current_swap_id == swap_id {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Watcher blocked during error retries

Medium Severity

The background watcher skips cancel/refund whenever SwapManager still lists a swap as running, but failed swaps now stay registered through indefinite exponential backoff retries. If bob::run keeps erroring before making progress, the watcher never runs its safety refund even when cancel timelocks expire.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 90c4b2f. Configure here.

@Einliterflasche
Copy link
Copy Markdown

  • stale swaps still get shown on the Swaps tab even when I clicked "do not auto resume"
  • I'd expect there to be one mock control ui thing for each swap, immediately as part of that swap widget. so that we can test multipe swaps at the same time, controlling each on it's own

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants